Shouln't this be 'one blob per userid ?

It's both. It's one per clientid and by the user that is authenticated. I'll change it slightly to maybe make it clearer.

Should not the client decode and enode the JSON data so we are just dealing with strings here?

The API automatically converts returns to JSON values so if we don't decode this into an object we stringify it twice.

Yes, of course, you are right.

But I'm still concerned about this. As I've discovered elsewhere, converting between javascript objects and arrays and php arrays and associative arrays via JSON can cause some screw-ups. Let me think about this some more.

In fact, given the existing API structure, we do want to encode it twice when returning the string to the client. The client encodes its javascript object into a JSON string and expects to get back the same string. The string is sent to the server and stored unchanged in the database and when it gets sent back it is encoded again, as you said, by the json_encode in index.php and then decoded by the response.json() in api.js. The client has to decode it again to recover the original javascript object.
So we don't want to do any encoding or decoding in v1_storage.inc.

Would the client not already have been validated before we get here?

It would. I did it here too to ensure it's enforced at the data layer not just the user input layer if this is used elsewhere for some reason.

Wouldn't it make sense to return the timestamp (something the client could use for caching) rather than the JSON blob that was sent by the client?

The timestamp is arguably just as useless as the JSON blob because it's always now(). I was uncertain what a good return value would be so I modeled what we do for PUTs for word lists -- and what the product at my job does for PUTs and PATCHes -- which is to return the object that was actually serialized.

Open to ideas / thoughts on what API clients should "expect" from a successful PUT / PATCH / etc beyond the HTTP status code.

The timestamp is arguably just as useless as the JSON blob because it's always now().

Sure as it represents the last modified timestamp from the server's perspective.

However I do think there is a lot more value to returning this compared to the blob that was sent and is known to the client. The timestamp would allow the client to implement some caching (e.g. don't check with the server if the timestamp is fresh enough), lower the bandwidth needed (by return the JSON object on a get request if the timestamp doesn't match, useful for mobile devices) or potentially even allow for delta (this would need to switch to some event based logic). Arguably, neither of those scenarios applies here, but those are enabled by returning the timestamp 😄

Open to ideas / thoughts on what API clients should "expect" from a successful PUT / PATCH / etc beyond the HTTP status code.

I don't think we need to return anything from the API on success: the return code says if it was successful and we can return an optional error field to return the cause of errors to the client. Those 2 are what a minimal client would expect. We can always add more as we understand our needs more.

We should move defining $value where need it before the loop on l.69.

I am having a hard time understanding the purpose of the API client storage keys. If the end goal is to allow any client to store this configuration down the road (which means that a user could trivially get one of our keys), is this some temporary measure to prevent arbitrary storage from storing JSON in our DB?

You have the gist of it, but it's not temporary.

Keep in mind that every user has de facto API access via PHP sessions -- that's how the Page Browser users the API and how the new proofreading UI will too. If we don't have some restriction, any logged in user could fill up our server with random JSON blobs. The clientid means that each user can have only at most len(API_CLIENT_STORAGE_KEYS) JSON blobs and those have a maximum size.

Perhaps "client" is the wrong word for this? I am using it in the "JS client that uses the APIs". Would "app" or some other word be clearer?

I'm having trouble visualizing it, too. Part of the discussion has centered around not only persisting settings across browsers and devices, but allowing different browsers on different devices to have settings specific to those devices (for example, if one device has a large monitor, the user may prefer the side-by-side version of the interface, but prefer top-to-bottom on a device with a smaller monitor).

My initial impression of "per client per user" was that multiple devices was covered because each user might be using different clients, thus, each user might have multiple entries, so I'm wondering if I'm just misunderstanding the terminology.

The JS client (ie: new proofreading interface) will need to figure out which settings it should store in the local browser cache (and only available to that browser -- like window size) and which one it should store on the server (and available to any browser device -- like font family). All this endpoint does is handle saving and returning whatever the JS client sends it. We have to figure out the other parts but that's all done within the JS client.

The clientid means that each user can have only at most len(API_CLIENT_STORAGE_KEYS) JSON blobs and those have a maximum size.

I am not seeing any validation on the number of stored blobs, we only check that the client id is allow-listed but besides that the constraints on the DB would allow unbounded storage. Those are trusted clients so it doesn't seem to be a concern, just curious about this.

Perhaps "client" is the wrong word for this? I am using it in the "JS client that uses the APIs". Would "app" or some other word be clearer?

Mhh, is the idea to separate our browser users from other "app" that programmatically call our API?

You have the gist of it, but it's not temporary.

OK, as a side question since we need a logged in users to store the information, would it make sense to use the user rather than an extra key? Or are we thinking that some "apps" will backfill/mutate a lot of users' keys? I am probably missing something rather obvious here as I don't have much context on the new UI.

The API automatically converts returns to JSON values so if we don't decode this into an object we stringify it twice.

You have the gist of it, but it's not temporary.

Keep in mind that every user has de facto API access via PHP sessions -- that's how the Page Browser users the API and how the new proofreading UI will too. If we don't have some restriction, any logged in user could fill up our server with random JSON blobs. The clientid means that each user can have only at most len(API_CLIENT_STORAGE_KEYS) JSON blobs and those have a maximum size.

The timestamp is arguably just as useless as the JSON blob because it's always now(). I was uncertain what a good return value would be so I modeled what we do for PUTs for word lists -- and what the product at my job does for PUTs and PATCHes -- which is to return the object that was actually serialized.

Open to ideas / thoughts on what API clients should "expect" from a successful PUT / PATCH / etc beyond the HTTP status code.

It would. I did it here too to ensure it's enforced at the data layer not just the user input layer if this is used elsewhere for some reason.

In this case it is already encoded by the client so we don't want to encode it again.

In fact, given the existing API structure, we do want to encode it twice when returning the string to the client. The client encodes its javascript object into a JSON string and expects to get back the same string.

No, that's not how it works.

The Client isn't sending us an opaque string, it's sending us a JSON object. That comes through as a string and the ApiRouter decodes that string into an object -- it does this for all request bodies and it's how all of the functions work. Our ApiClientStorage get() and set()s take strings, so we have to encode that back into a string to store it.

Client sends JSON in request -> API converts to object -> v1_storage.inc converts back to string -> ApiClientStorage persists string

When we get() it from ApiClientStorage the class returns a string. But returns from API router functions needs to be an object, because the API will encode that into a string, so we need to decode it into an object first.

ApiClientStorage returns string -> v1_storage.inc converts back to object -> API converts object to string -> Client receives JSON in return

In fact, given the existing API structure, we do want to encode it twice when returning the string to the client. The client encodes its javascript object into a JSON string and expects to get back the same string.

No, that's not it works.

The Client isn't sending us an opaque string, it's sending us a JSON object. That comes through as a string and the ApiRouter decodes that string into an object -- it does this for all request bodies and it how all of the functions work. Our ApiClientStorage get() and set()s take strings, so we have to encode that back into a string to store it.

Client sends JSON in request -> API converts to object -> v1_storage.inc converts back to string -> ApiClientStorage persists string

When we get() it from ApiClientStorage the class returns a string. But returns from API router functions needs to be an object, because the API will encode that into a string, so we need to decode it into an object first.

ApiClientStorage returns string -> v1_storage.inc converts back to object -> API converts object to string -> Client receives JSON in return

I see why we are at cross purposes; you are envisaging the client sending a javascript object whereas I was assuming the client would encode it first and send a JSON object. Sorry that's not what you said but in my scenario the client encodes the javascipt object into a JSON object and it gets doubly encoded in transport but decoded back into a JSON object by api_get_request_body() so we can put it straight into the database.
p.s. I think the api_get_request_body() is a bit confusing because when it says return $json, the thing it is returning generally isn't JSON but what was originally provided as data to the ajax function. (though in my scenario it now is).

p.s. I think the api_get_request_body() is a bit confusing because when it says return $json, the thing it is returning generally isn't JSON but what was originally provided as data to the ajax function. (though in my scenario it now is).

Yes, that's totally fair -- api_get_request_body() returns a decoded object/array, not a string - $json_object would be clearer.

And yes, the client is encoding the object into JSON -- we do that already in the ajax function:

    if (upperCaseMethod !== "GET") {
        // POST or PUT
        options.headers["Content-Type"] = "application/json";
        options.body = JSON.stringify(data);
    }

The client just needs to pass in the object it wants persisted as $data to ajax() and it'll all just work.

I see what you mean. But I think there could be some problems with that approach. The original javascript object will get encoded into JSON and will get decoded into a PHP object and then re-encoded into JSON and the reverse on the way back. PHP objects don't correspond exactly to javascript objects. As we have seen, a php associative array which happens to have consecutive numerical keys or is empty is encoded as an ordinary array. What we get back may not be the same as what we started with.
The other approach of having the client make a JSON object with JSONstringify and then sending that (it would get doubly encoded) would, I imagine, be safer since we are only json-encoding and -decoding strings in the server. v1_storage would then not need to do any encoding or decoding but the client would have to encode and decode so the number of codings would end up much the same overall.

No, I don't want to do that. When we save a page from the PI we send a string from the client and the same string gets put into the db. What I'm proposing is sending a JSON string in the same way and this same JSON string gets put into the db as JSON.

The page is a string inside a JSON object -- it's not a naked string. So what you're proposing is having a JSON string as a value of a JSON object:

{
    "data": "{\"key\": \"value\"}"
}

And then the API would extract data out of the payload and pass the string into the ApiClientStorage class.

That's a weird and unexpected interface. No one does that -- just pass the JSON object 😁

It's getting late here now. I'll consider it later but have a happy Christmas.

I've put up a commit that allows a routed function to get the non-deserialized input and output non-serialized output. This is less hacky than I thought it would be. It's in a separate commit for evaluation / discussion.

Essentially yes but without the wrapping. However I like your idea of bypassing the encoding and decoding in the lower levels of the API much better.

But I'm still concerned about this. As I've discovered elsewhere, converting between javascript objects and arrays and php arrays and associative arrays via JSON can cause some screw-ups. Let me think about this some more.

In fact, given the existing API structure, we do want to encode it twice when returning the string to the client. The client encodes its javascript object into a JSON string and expects to get back the same string. The string is sent to the server and stored unchanged in the database and when it gets sent back it is encoded again, as you said, by the json_encode in index.php and then decoded by the response.json() in api.js. The client has to decode it again to recover the original javascript object.
So we don't want to do any encoding or decoding in v1_storage.inc.

This function could stay in index.php

I moved this here to keep the encoding and decoding together. And because I had to move the encoding here this came along with it.

That seems reasonable. I've had some further thoughts about this and deleted my other ananswered comments which had overlooked a change you made in index.php. It's difficult to see the whole picture in this github view so I dowloaded the files to look at them more carefully.

We wouldn't need this if it was incorporated as above.

Because I wanted to keep the encoding and decoding together and they got moved in here, breaking them out into clear functions makes it clearer what's going on IMHO.

The clientid means that each user can have only at most len(API_CLIENT_STORAGE_KEYS) JSON blobs and those have a maximum size.

I am not seeing any validation on the number of stored blobs, we only check that the client id is allow-listed but besides that the constraints on the DB would allow unbounded storage. Those are trusted clients so it doesn't seem to be a concern, just curious about this.

Perhaps "client" is the wrong word for this? I am using it in the "JS client that uses the APIs". Would "app" or some other word be clearer?

Mhh, is the idea to separate our browser users from other "app" that programmatically call our API?

You have the gist of it, but it's not temporary.

OK, as a side question since we need a logged in users to store the information, would it make sense to use the user rather than an extra key? Or are we thinking that some "apps" will backfill/mutate a lot of users' keys? I am probably missing something rather obvious here as I don't have much context on the new UI.

The timestamp is arguably just as useless as the JSON blob because it's always now().

Sure as it represents the last modified timestamp from the server's perspective.

However I do think there is a lot more value to returning this compared to the blob that was sent and is known to the client. The timestamp would allow the client to implement some caching (e.g. don't check with the server if the timestamp is fresh enough), lower the bandwidth needed (by return the JSON object on a get request if the timestamp doesn't match, useful for mobile devices) or potentially even allow for delta (this would need to switch to some event based logic). Arguably, neither of those scenarios applies here, but those are enabled by returning the timestamp 😄

Open to ideas / thoughts on what API clients should "expect" from a successful PUT / PATCH / etc beyond the HTTP status code.

I don't think we need to return anything from the API on success: the return code says if it was successful and we can return an optional error field to return the cause of errors to the client. Those 2 are what a minimal client would expect. We can always add more as we understand our needs more.

Do you need this line now?